Skip to content

Conversation

@AlexLanzano
Copy link
Contributor

@AlexLanzano AlexLanzano commented Aug 5, 2025

Flash RAM Sim changes

Currently whFlashRamsim_Init uses malloc to allocate the simulated NVM space in RAM. This may be cumbersome for environments where dynamic memory allocation is not implemented or unavailable. This PR implements the following to address this:

Integer sign fixes

There were multiple instances uint32_t variable, return value, and/or field that gets set to an error code which are negative values. The TI arm compiler throws an error for this that can't be turned off or ignored. In this PR I changed these occurrences to use a signed integer instead.

Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent catch on the signed values. I found some other messages that had errors in them too, but I'll fix those seperately.

Consider eliminating malloc() within this library by updating the test/examples/tools to malloc instead.

Just need a few parameter validations and I think we are probably good!

Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of lingering free()'s. Looks great!

Please pull before you do a final push to resolve the conflict in test_cert.

@billphipps billphipps requested a review from Copilot August 19, 2025 12:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes malloc dependencies from the Flash RAM Simulator library and fixes integer sign mismatches for error codes. The changes enable usage in environments without dynamic memory allocation and address TI ARM compiler warnings about assigning negative error codes to unsigned integers.

  • Adds a memory field to whFlashRamsimCfg to allow user-provided memory allocation instead of malloc
  • Changes uint32_t return code fields to int32_t in SHE message structures to properly handle negative error values
  • Updates the nfObject_Offset function to return error codes properly instead of mixing offsets with error values

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
wolfhsm/wh_flash_ramsim.h Adds memory field to configuration structure
wolfhsm/wh_message_she.h Changes return code fields from uint32_t to int32_t
src/wh_flash_ramsim.c Removes malloc dependency and uses user-provided memory
src/wh_nvm_flash.c Fixes nfObject_Offset to return errors via output parameter
test/*.c Updates test files to provide static memory buffers
examples/*.c Updates examples to provide static memory buffers
benchmark/*.c Updates benchmark to provide static memory buffer

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@billphipps billphipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks!

@billphipps billphipps dismissed bigbrett’s stale review August 19, 2025 17:22

Changes have been implemented.

@billphipps billphipps merged commit 19e88be into wolfSSL:main Aug 19, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants